LoginSignup
icoke
@icoke

Are you sure you want to delete the question?

Leaving a resolved question undeleted may help others!

このプログラムを改修するにあたっていくつか質問したいです

解決したいこと

paiza というサイトの問題集で以下の問題を以下のプログラムで解きました。
そして、問題自体はクリアできたのですが、気になったことがあったので質問させてください。

ソースコードの詳細
using System;
using System.Runtime.CompilerServices;

// n が縦の長さで、インデックスは i を割り当ててる
// m が横の長さで、インデックスは j を割り当ててる

namespace Island {
    static class Program {
        static void Main ( ) {
            Read.InitNum(out int n, out int m); // 初期化数の読み込み
            int[,] map = new int[n, m]; // 初期化
            Read.Island(map, n, m); // map データの読み込み
            int count = Search.Bigin(map, n, m); // 走査して、島の数を格納
            Console.WriteLine(count); // 表示
        }
    }
    // 読み込みメソッドをまとめたクラス
    static class Read {
        public static void InitNum ( out int n, out int m ) {
            string[] strs = Console.ReadLine().Split();
            m = int.Parse(strs[0]);
            n = int.Parse(strs[1]);
        }

        public static void Island ( int[,] map, int n, int m ) {
            for ( int i = 0; i < n; i++ ) {
                string[] strs = Console.ReadLine().Split();
                for ( int j = 0; j < m; j++ ) {
                    map[i, j] = int.Parse(strs[j]);
                }
            }
        }
    }

    // 走査とカウントに必要なメソッドをまとめたクラス
    static class Search {
        public static int Bigin ( int[,] map, int n, int m ) {
            // 島コード:陸地がない場合は 0 、陸地がある場合は 1 で map が読み込まれているので
            // 2, 3, 4, 5 と分類コードを振り分けていく
            int code = 2;
            // map 全体を走査して map[i, j] が未分類コード 1 の時だけ
            // 再帰的に連続する陸地を同じ分類コードに塗っていく
            for ( int i = 0; i < n; i++ ) {
                for ( int j = 0; j < m; j++ ) {
                    if ( map[i, j] == 1 ) {
                        Recursive(code, map, i, j, n, m);
                        code++;
                    }
                }
            }
            // 最後の分類コードから初期値 2 を引いた数が島の数になる
            return code - 2;
        }

        // 上下左右でメソッドを分けて効率化を図るか
        // このままにしてすっきりしたコードを保つか
        private static void Recursive ( int code, int[,] map, int i, int j, int n, int m ) {
            if ( i >= 0 && j >= 0 && i < n && j < m && map[i, j] == 1 ) {
                map[i, j] = code;
                Recursive(code, map, i + 1, j, n, m ); // 下
                Recursive(code, map, i - 1, j, n, m ); // 上
                Recursive(code, map, i, j + 1, n, m ); // 右
                Recursive(code, map, i, j - 1, n, m ); // 左
            }
        }
    }
}

質問 関数を呼び出すたびに同じ変数を引き渡してていいのか?

 map, n, mをどこでも使っているので、C言語で使われるグローバル変数のようにしていちいち引数で渡す手間を省きたい。
そこで静的なクラスpublic static class GVを作ってGV.と参照する。

実装した静的なクラスの詳細
public static class GV { // GlobalVariables
    public static int[,] Map { get; set; }
    public static int N { get; set; }
    public static int M { get; set; }
}

のもいいですが、いちいちGV.をつけるのも面倒です。
ほかの方法を調べると{ get; set; }を使えばフィールドを設定できるらしい?ということで、試しに書いてみました。
class Readclass Searchにそれぞれ以下のコード

追加したプロパティの詳細
public static int[,] Map {
    get { return GV.Map; }
    set { GV.Map = value; }
}
public static int N {
    get { return GV.N; }
    set { GV.N = value; }
}
public static int M {
    get { return GV.M; }
    set { GV.M = value; }
}

を書き加えて以下のようなソースコードにしました。

改修したソースコードの詳細
using System;
using System.Runtime.CompilerServices;

namespace Island {
    static class Program {
        static void Main ( ) {
            Read.InitNum();
            Read.Island();
            int count = Search.Bigin();
            Console.WriteLine(count);
        }
    }

    // グローバル変数のように扱うクラス
    public static class GV {
        // N が縦の長さで、インデックスは i を割り当ててる
        // M が横の長さで、インデックスは j を割り当ててる
        public static int[,] Map { get; set; }
        public static int N { get; set; }
        public static int M { get; set; }
    }

    // 読み込みメソッドをまとめたクラス
    static class Read {
        public static int[,] Map {
            get { return GV.Map; }
            set { GV.Map = value;  }
        }
        public static int N {
            get { return GV.N; }
            set { GV.N = value; }
        }
        public static int M {
            get { return GV.M; }
            set { GV.M = value; }
        }

        public static void InitNum ( ) {
            string[] strs = Console.ReadLine().Split();
            M = int.Parse(strs[0]);
            N = int.Parse(strs[1]);
            Map = new int[N, M];
        }

        public static void Island ( ) {
            for ( int i = 0; i < N; i++ ) {
                string[] strs = Console.ReadLine().Split();
                for ( int j = 0; j < M; j++ ) {
                    Map[i, j] = int.Parse(strs[j]);
                }
            }
        }
    }

    // 走査とカウントに必要なメソッドをまとめたクラス
    static class Search {
        public static int[,] Map {
            get { return GV.Map; }
            set { GV.Map = value; }
        }
        public static int N {
            get { return GV.N; }
            set { GV.N = value; }
        }
        public static int M {
            get { return GV.M; }
            set { GV.M = value; }
        }
        public static int Bigin ( ) {
            // 島コード:陸地がない場合は 0 、陸地がある場合は 1 で map が読み込まれているので
            // 2, 3, 4, 5 と分類コードを振り分けていく
            int code = 2;
            for ( int i = 0; i < N; i++ ) {
                for ( int j = 0; j < M; j++ ) {
                    if ( Map[i, j] == 1 ) {
                        // map 全体を走査して map[i, j] が未分類コード 1 の時だけ
                        // 再帰的に連続する陸地を同じ分類コードに塗っていく
                        Recursive(code, i, j);
                        code++;
                    }
                }
            }
            // 最後の分類コードから初期値 2 を引いた数が島の数になる
            return code - 2;
        }

        // 上下左右でメソッドを分けて効率化を図るか
        // このままにしてすっきりしたコードを保つか
        private static void Recursive ( int code, int i, int j ) {
            if ( i >= 0 && j >= 0 && i < N && j < M && Map[i, j] == 1 ) {
                Map[i, j] = code;
                Recursive(code, i + 1, j );
                Recursive(code, i - 1, j );
                Recursive(code, i, j + 1 );
                Recursive(code, i, j - 1 );
            }
        }
    }
}

しかしこれだと、classの数が増えるたびに同じコードを書き加えなきゃいけないのと、そもそも、とりあえず動いてはいるけど{ get; set; }の使い方がこれで正しいのか分からないです。

ということで、{ get; set; }の使い方が正しいか、
class GVにする、{ get; set; }を上記のように使う、以外で何か方法があるか知りたいです。

2

5Answer

グローバル変数 → シングルトンパターンへ(可能ならスレッドセーフなクラスにする)
https://atmarkit.itmedia.co.jp/ait/articles/1711/29/news029.html

シングルトンにした後のイメージは以下

static class Read {
 private static GV _gv = GV.GetInstance();

static class Search {
 private static GV _gv = GV.GetInstance();

使う時のイメージは以下

public static int Bigin ( ) {
 int code = 2;
     for ( int i = 0; i < _gv.N; i++ ) {
        for ( int j = 0; j < _gv.M; j++ ) {
 
1

Comments

  1. @icoke

    Questioner

    調べてみます

  2. @icoke

    Questioner

    ちょっと難しいですが、ざっくりと動的クラスを静的に使うことで継承を可能にしているという認識で会ってますか?
    今まで静的クラスを継承したいと思ったことがないので知りませんでしたが、いつか役に立ちそうなので覚えておこうと思います。

  3. 誤解を与えてしまったかもしれない点を訂正しました

    new GV(); → GV.GetInstance();
    

    ロガーや設定ファイル等どこでも使うようなものはシングルトンパターンで実装されるケースが多いです。
    アクセサ(get,set)の他にインスタンスを返す関数(今回はGetInstance())などを追加することで
    唯一無二のオブジェクトを使用できる形になります。
    変数にするほどでもない場合は以下のような使い方になる。

    GV.GetInstance().N
    
  4. @icoke

    Questioner

    正直に言うと全然分かりません。
    多分前提知識が足りてないんだと思います。
    もうちょい知識をつけてから勉強しなおそうと思います。

ある程度気づいていると思いますが、いろいろ工夫しても結局「いちいちGVつける」方が楽。
先頭行にusing static GV;と書いておくとGVをつけなくてもよくなりますがusing static禁止令なんて記事が書かれるくらいなので
「いちいちGVつける」が一番ましだと言っていいと思います。

1

Comments

  1. @icoke

    Questioner

    そうなんですね...。確かにその通りですね。
    記事を読んでみましたが、C言語的な発想でオブジェクト指向がまだ身にしみついていないとか言われてしまいました。実際C言語上がりなので図星です。もっとオブジェクト指向を理解できるように勉強しようと思います。

map, n, mをどこでも使っているので、グローバル変数のようにしていちいち引数で渡す手間を省きたい。

C#的なやり方をするなら、まずグローバル変数という考え方をやめて、クラスのインスタンスを受け渡しするようにした方がいいと思います。クラス名やクラスメンバ名も、役割に応じてきちんとわかりやすい名前を付けるクセをつけましょう。
staticを使いまくらなくても、こんな感じでいいのでは。

using System;

class Program
{
    /// <summary>
    /// 島データ
    /// </summary>
    public class IslandData
    {
        public int[,] Map { get; set; }
        public int Height { get; set; }
        public int Width { get; set; }
    }

    /// <summary>
    /// コンソール入力受信用クラス
    /// </summary>
    public class ConsoleReader
    {
        public static IslandData GetIslandData()
        {
            // TODO: コンソール入力からIslandDataのインスタンスを作成する処理の実装
            return null;
        }
    }

    /// <summary>
    /// 島数計算クラス
    /// </summary>
    public class IslandCounter
    {
        private IslandData _data;

        public IslandCounter(IslandData data)
        {
            _data = data;
        }

        public int GetCount()
        {
            // TODO: 島数計算処理の実装

            // メンバの_dataを使ってなんかする

            return 0;
        }
    }

    public static void Main()
    {
        IslandData data = ConsoleReader.GetIslandData();
        IslandCounter counter = new IslandCounter(data);
        int count = counter.GetCount();

        Console.WriteLine(count);
    }
}

質問文のGVにあたるのがIslandDataで、それを受け渡しするようにしています。
データ作成部を分離しておけば、実際にコンソール入力を受け取らずとも、ダミーのデータを作成するメソッドを作成し、固定データを返すような作りにもできます。

1

Comments

  1. @icoke

    Questioner

    そうですね。グローバル変数という考え方はやめようと思います。あと命名規則とかクラスの構造とか参考にさせていただきます。

解決したということでクローズしようと思います。
ありがとうございました。

1

いちいちGV.をつけるのも面倒です

C# 詳しくないのですが,classを入れ子にした場合には付けなくてもいけるっぽいですよ.

namespace Test
{
    class Program
    {
        private static int TheStaticVal = 0;

        //入れ子にしたclass
        class Read
        {
            public static void Exec()
            {// Program.TheStaticVal という形で書かなくてもOKっぽい.
                TheStaticVal = 77;
            }
        }

        //Main
        static void Main(string[] args)
        {
            Read.Exec();
            Console.WriteLine( TheStaticVal );  //77と表示される
        }
    }
}
1

Comments

  1. @icoke

    Questioner

    それいいかもしれないですね。
    また試してみます。

Your answer might help someone💌